fix(core): raise the nx package vitest timeout to cover the graph recompute spec - #36850
Merged
Conversation
…ompute spec project-graph-incremental-recomputation.spec.ts times out at exactly 35000ms on CI on every run that actually executes it: three runs in a row clipped at 35,009 / 35,018 / 35,070ms. Measured with the limit raised, the slowest test needs 42.4s on CI hardware, and its neighbours land at 32s and 29s. Locally the same test swings between 7s and 23s depending on pool load, so a 35s ceiling is inside the spec's own variance once CI's roughly 5x slowdown is applied. Master rarely shows it because most commits do not touch packages/nx and nx:test is served from cache. Any branch that busts that cache runs the spec every push and hits the limit. The jest preset carried the same 35s before the vitest move, so this is not a transform regression; the spec has been close to the line for a while and only surfaces on branches that execute it repeatedly.
✅ Deploy Preview for nx-docs ready!
To edit notification comments on pull requests, go to your Netlify project configuration. |
✅ Deploy Preview for nx-dev ready!
To edit notification comments on pull requests, go to your Netlify project configuration. |
Contributor
|
View your CI Pipeline Execution ↗ for commit 62c3e5e
☁️ Nx Cloud last updated this comment at |
barbados-clemens
approved these changes
Aug 30, 2026
FrozenPandaz
added a commit
that referenced
this pull request
Aug 31, 2026
…prefix the wire protocol (#36838) ## Current Behavior The daemon mirrors the client's wire format when serializing a response, with no fallback if that format cannot carry the payload. A large `HASH_TASKS` response kills the daemon: ``` Serializing response for HASH_TASKS message in json mode RangeError: Invalid string length at serializeUnserializedResult (daemon/server/server.js:514:21) at handleResult ``` The client side already had a JSON/v8 fallback, but it only covers messages sent to the daemon. Nothing covered responses. The throw escapes an un-awaited async callback, so it becomes an unhandled rejection and terminates the process rather than failing one request. Messages were also framed with a trailing `NX_MSG_END` delimiter and carried as latin1 strings, so every message was capped at Node's max string length of 536,870,888 characters in three places: `.toString('binary')` on a v8 buffer, the response concatenation, and the client's accumulating string. ## Expected Behavior Daemon responses fall back to the other format instead of throwing, and messages are no longer bounded by the max string length. Two commits: 1. **Framing.** Messages carry an `NX_MSG_<byteLength>:` header and travel as buffers end to end. Buffers sit outside the V8 heap, so a large response stops counting against `--max-old-space-size`. Framing is O(1) per message instead of a scan, so a payload containing the framing marker can no longer desynchronize the stream, and a complete message sharing a chunk with an incomplete one is delivered immediately rather than held until a chunk ends on a boundary. An incomplete message waits for its remaining bytes indefinitely — there is deliberately no idle timer, since Node runs timer callbacks before the poll phase, so a timer could discard bytes that had already arrived — while a runaway declared length is bounded by `NX_MAX_MESSAGE_SIZE`, and a header that does not parse fails the stream instead of hanging. 2. **Fallback.** `serializeWithFallback(data, preferred)` replaces `serializeUnserializedResult` and backs both directions. This also fixes a precedence bug it surfaced: `serialize()` branched on `force === 'v8' || isV8SerializerEnabled()`, so `force: 'json'` was ignored whenever `NX_USE_V8_SERIALIZER` was set. `processInBackground` forces JSON precisely because its payloads cannot be v8-cloned, so it paid a failed serialization and a spurious warning on every call under that env var. The format is detected per message after framing rather than per connection, because a single chunk can carry a JSON streaming progress message and a v8 response together. Responding in a format the client did not send is already safe: streaming progress messages have always been serialized from the daemon's own preference rather than the requesting client's. ### Why both changes ship together Measured on a `HASH_TASKS`-shaped payload, v8 output is 2.4% smaller than JSON. With a string transport the fallback only widens the working range from about 537MB to about 550MB, which stops the crash without giving the reporting workspace headroom. Opting into `NX_USE_V8_SERIALIZER` was not enough for them either. ### Compatibility The wire format changes in both directions across the four channels that share the framing: daemon client and server, plugin worker IPC, and pseudo-IPC. Version skew is already covered by the `nxVersion` handshake in `daemon/cache.ts`, which throws before any bytes are exchanged, so a new client never talks to an old daemon. ### Testing `consume-messages-from-socket.spec.ts` covers framing, header splits, desync, the size ceiling, the absence of idle timers, a payload whose bytes contain the framing marker, and three round-trips over a real unix socket where the OS picks the chunk boundaries. The fallback tests avoid allocating 512MB by using inputs each format rejects: `{value: 1n}` fails JSON, `{fn(){}}` fails v8. 3178 tests pass across `daemon`, `utils`, `project-graph/plugins`, and `tasks-runner`, with no new failures. ### Not affected `serializeResult()` builds the `REQUEST_PROJECT_GRAPH` response by string concatenation rather than going through this path. Its only other caller passes `(error, null, null)`. The project graph does not carry per-file data, so neither payload is anywhere near the limit that the task hash details hit. ### Stacked on #36850 This PR is based on #36850, which raises the `packages/nx` vitest timeout. `project-graph-incremental-recomputation.spec.ts` needs 42.4s on CI against the previous 35s limit and fails on any branch that executes it; this branch touches `packages/nx`, so it runs the spec every push. Merge #36850 first, then retarget this PR to `master`. The commit range here is only the nine daemon commits; the timeout change is not part of this diff. ### CI note Intermittent `Command timed out after 300s` failures on rollup builds in `e2e-react` / `e2e-remix` / `e2e-rollup` are the pre-existing flake tracked in #36794, not this change. That flake predates this branch, hits about 4% of master runs, and the dump there showed the surviving process is rollup's own CLI while the daemon round-trip finished in milliseconds. This branch touches `packages/nx`, so every e2e task runs rather than reading from cache, which is why it shows up more often here. ## Related Issue(s) Fixes NXC-4901 <!-- polygraph-session-start --> --- <p><a href="https://app.trypolygraph.com/orgs/6a061dcb561c062131116eca/sessions/daemon-serializer-920ef087">View Polygraph session ↗</a></p> <!-- polygraph-session-end --> --------- Co-authored-by: FrozenPandaz <jasonjean1993@gmail.com>
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Current Behavior
project-graph-incremental-recomputation.spec.tstimes out at exactly 35,000ms on CI on every run that executes it. Three consecutive runs on one branch clipped at 35,009ms, 35,018ms and 35,070ms on the same test. Master rarely shows it: most commits do not touchpackages/nx, sonx:testis served from cache, and 9 of the last 12 master commits never ran the spec.Expected Behavior
The spec has enough budget to finish on CI hardware. With the limit raised for measurement, the slowest test needs 42.4s and its neighbours land at 32s and 29s; the whole suite went green with no other change. Locally the same test swings between 7s and 23s depending on pool load, so a 35s ceiling sits inside the spec's own variance once CI's roughly 5x slowdown is applied. 90s clears the measured worst case by 2x.
This is not a vitest transform regression: the jest preset carried the same 35s before #36754.
Related Issue(s)
Unblocks #36838, which is stacked on this branch and will be retargeted to
masteronce this merges.